- 
                Notifications
    You must be signed in to change notification settings 
- Fork 151
[CC-33024] Remove the deprecated fields of CockroachDB operator #560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
13ac257    to
    b01e115      
    Compare
  
    | clusterSettings: ~ | ||
| # timestamp captures the annotation timestamp used for rolling restarts. | ||
| timestamp: "2021-10-18T00:00:00Z" | ||
| # resources captures the resource requests and limits for CockroachDB pods. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we need to remove the whole resources section here.
        
          
                build/templates/cockroachdb-parent/charts/cockroachdb/values.yaml
              
                Outdated
          
            Show resolved
            Hide resolved
        
      0f4e15d    to
    1238344      
    Compare
  
    1238344    to
    f886c28      
    Compare
  
    | # If specified, podTemplate is merged with the default pod specification, with settings in podTemplate taking precedence. | ||
| # This can be used to add or update containers, volumes, and other settings of the CockroachDB pod. | ||
| podTemplate: {} | ||
| podTemplate: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: need to fix the indentation (of comments) within the podTemplate section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
162b3a6    to
    647a3a0      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prafull01 - as discussed, let's add a changelog entry for these changes and seek @jhlodin 's review for it.
647a3a0    to
    3ab1dae      
    Compare
  
            
          
                CHANGELOG.md
              
                Outdated
          
        
      | All notable changes to this project will be documented in this file. | ||
|  | ||
| ## [cockroachdb-parent-25.3.3-preview+1] 2025-10-29 | ||
| - Remove the deprecated fields and start using the podTemplate fields. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should list the deprecated fields, at least at a high level. Like:
- Removed the following deprecated fields in favor of `podTemplate`:
    - `crdbCluster.resources.*`
    - `crdbCluster.podLabels.*`
    - `crdbCluster.topologySpreadConstraints.*`
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'll also make it easier for me to identify which parts of the docs need to be updated to remove reference to deprecated, so if for some reason we don't want to list this out in the release note I could still use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand to know before hand which fields are deprecated. However this change would make the CHANGELOG.md very log. Should we add that description here or I can add that in the ticket so it would be easier for you to write the documentation. This will add additional 10-12 line specific to deprecation in CHANGELOG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think 10-12 lines is acceptable. I've put similar lists in breaking changes/release notes in the past and AFAIK it was received well - people appreciate having the detail when you introduce a breaking change like deprecation/removal, and if they need to ctrl+F a field it's useful having it in the list. Best if we can have sub-bullets for each field like I proposed in my first comment, rather than individual release notes for every deprecation.
3ab1dae    to
    e0b0875      
    Compare
  
    1c7e18c    to
    7403e57      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One quick suggestion, otherwise LGTM
        
          
                CHANGELOG.md
              
                Outdated
          
        
      | All notable changes to this project will be documented in this file. | ||
|  | ||
| ## [cockroachdb-parent-25.3.3-preview+1] 2025-10-29 | ||
| - Remove the deprecated fields and start using the podTemplate fields. Following fields are deprecated: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Remove the deprecated fields and start using the podTemplate fields. Following fields are deprecated: | |
| - Remove the following deprecated fields in favor of the corresponding podTemplate fields: | 
7403e57    to
    e2c081c      
    Compare
  
    
No description provided.